Deploy build 8083 of cms-fronts::facia-tool to CODE

From guardian/actions-riff-raff.

Hmmmmmmmm. I see the logic of using Either here, for its simplicity and being well-understood, but it is a bit of a short-term fudge (just because there are two now, doesn't mean we might not want more in the future). CoProduct seems a much more logical choice IMO, but I take the point that it may represent more complexity/unfamiliarity than is warranted right now (and I think it's a safe assumption that upgrading to Scala3 is out of the question for a while, given the size of the codebase!).

I'm not scala expert though and would welcome an eye from someone who has more direct experience of shapeless etc. - @emdash-ie @JustinPinner what do you think?

It might be worth adding some notes to the doc about the logic for the differences between com.gu.facia.client.models.Trail (from the path I assume it's a Thrift model) and model.editions.EditionsClientCard (from the path I assume it's not a Thrift model) e.g. why is one Thrift and one not, what are the field differences between them, why can one not inherit from the other, etc.

I’m totally happy with this use of Either, and I don’t think using shapeless’ coproduct type would be a good idea: it’s far too awkward to work with for direct use. There’s nothing (beyond a dislike of boilerplate) stopping us from wrapping these in an extra ADT, which is how we can evolve this when we add a third type of thing:

object ClipboardCard {
  def apply(trail: Trail): ClipboardCard = TrailCard(trail)
  def apply(editionsCard: EditionsClientCard): ClipboardCard = EditionsCard(editionsCard)

  // presumably this could use TrailCard and EditionsCard directly, but I don’t know how offhand
  val reads: Reads[ClipboardCard] =
    JsPath.read[EditionsClientCard].map(ClipboardCard.apply) or
      JsPath.read[Trail].map(ClipboardCard.apply)

  // can we even get this and the Reads automatically once we’re using an ADT? Feels like it should be possible
  val writes =  new Writes[ClipboardCard] {
    override def writes(o: ClipboardCard): JsValue =
      o match {
        case TrailCard(trail) => Json.toJson(trail),
        case EditionsCard(editionsCard) => Json.toJson(editionsCard)
      }
  }

  implicit val format = Format(reads, writes)
}

sealed trait ClipboardCard
case class TrailCard(card: Trail) extends ClipboardCard
case class EditionsCard(card: EditionsClientCard) extends ClipboardCard

Thanks for the comments, all. @fredex42 yes, the third thing breaks this! @emdash-ie, that's a nice way of solving for that, and thanks for going to the trouble of writing out an example. I think in the back of my mind is the idea that I'd expect the number of entities to shrink, not grow, as we converge on a single representation of a trail/card across products, but this is a) a hunch and b) a hostage to fortune 😀

Sounds like we're happy enough to merge as-is, but if there's any additional feedback, let me know.

It might be worth adding some notes to the doc about the logic for the differences between com.gu.facia.client.models.Trail (from the path I assume it's a Thrift model) and model.editions.EditionsClientCard (from the path I assume it's not a Thrift model) e.g. why is one Thrift and one not, what are the field differences between them, why can one not inherit from the other, etc.

👍 1524285

Seen on PROD (merged by @jonathonherbert 6 minutes and 23 seconds ago) Please check your changes!